Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced isfunction & ismethod with isroutine #707

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

TheRealVira
Copy link
Contributor

This pretty much returns exactly the same as before, but with the benefit of including C functions/methods. You can read more about that here:

https://stackoverflow.com/a/17019998/6901146

This pretty much returns exactly the same as before, but with the benefit of including C functions/methods. You can read more about that here:

https://stackoverflow.com/a/17019998/6901146
@TheRealVira TheRealVira requested a review from a team as a code owner July 15, 2022 15:52
Copy link
Contributor

@KTAtkinson KTAtkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

Please make sure CI passes.

baseplate/clients/thrift.py Outdated Show resolved Hide resolved
baseplate/clients/thrift.py Outdated Show resolved Hide resolved
@TheRealVira
Copy link
Contributor Author

@KTAtkinson so... I can't re-run the workflow since I am a "First-time contributor". Would you please do me the honors to approve the run?

@TheRealVira
Copy link
Contributor Author

What do you think? Shall I adapt the test cases? (:

Copy link
Contributor

@KTAtkinson KTAtkinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved the run but it looks like it failed :)

@TheRealVira
Copy link
Contributor Author

@KTAtkinson I have replaced inspect with __dict__ - inspect apparently returns user-defined and built-in functions.

@TheRealVira
Copy link
Contributor Author

Awesome! @KTAtkinson you need anything more from my end for this PR? (:

Btw I'd really appreciate an "Open Sorcerer" trophy on my Reddit profile

@TheRealVira
Copy link
Contributor Author

💇‍♀️

@KTAtkinson
Copy link
Contributor

Nothing else needed from your end, we are working to get a critical feature out now, so I won't be able to release this for a bit.

@TheRealVira
Copy link
Contributor Author

💇‍♀️

@TheRealVira
Copy link
Contributor Author

hi @KTAtkinson it looks like @aba-ca-xi isn't available (anymore?). Could you please assign another reviewer?

@KTAtkinson KTAtkinson added the v2.6 Pull requests to be included in v2.6 label Apr 4, 2023
@TheRealVira
Copy link
Contributor Author

@maeivysea maybe?

@TheRealVira
Copy link
Contributor Author

@KTAtkinson when merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.6 Pull requests to be included in v2.6
Development

Successfully merging this pull request may close these issues.

2 participants